Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose Ruff's public API as a Python library #1407

Closed
wants to merge 4 commits into from

Conversation

squiddy
Copy link
Contributor

@squiddy squiddy commented Dec 27, 2022

See #659

  • Accept options (need to check what a good way here is, either just a partial dictionary similar to WASM, or proper classes with type hints?)
  • Configure and document testing
  • Include in CI
  • Build a wheel with binary and library
  • Finalize what the API exactly is (Expose Ruff's public API as a Python library #1407 (comment))


#[pyfunction]
fn check(contents: &str) -> PyResult<Vec<Message>> {
// TODO(rgerecke): Accept settings
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also accept an optional filename.

Copy link
Contributor Author

@squiddy squiddy Dec 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On that topic, do we mirror the current API 1:1, i.e. should this also resolve settings by looking into the filesystem?

There was some discussion on the issue itself, but no clear direction yet (at least for me).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we should. Similar to the Rust API (which doesn't accept settings... but should).

I think the rules are:

  • If settings are provided, use those.
  • Else, if filepath is provided, infer.
  • Else, use defaults.

@squiddy squiddy force-pushed the python-library branch 2 times, most recently from 75db8af to 7a91b3b Compare December 27, 2022 20:05
@squiddy squiddy force-pushed the python-library branch 4 times, most recently from de5576a to 6bd1f6b Compare December 28, 2022 08:46
maturin doesn't yet support packaging both a binary and a library into
the same package.

PyO3/maturin#368
@squiddy
Copy link
Contributor Author

squiddy commented Dec 28, 2022

Most pieces are in place. Since maturin doesn't support binary+library in one package (at least I didn't find a way), I had to switch to the lower-level setuptools-rust. I need to do some more CI adjustments for that, so I'll be moving that into a separate PR.

@charliermarsh
Copy link
Member

Sweet! I haven't reviewed yet, but @messense, do you know if Maturin can be used here to build a binary + library in one package?

@squiddy
Copy link
Contributor Author

squiddy commented Dec 28, 2022

I haven't reviewed yet

For the record, it's also not in a state where I'm waiting for a review. I can still do some cleaning up, adding docs and function signatures to the library, adjust CI etc.

@messense
Copy link
Contributor

do you know if Maturin can be used here to build a binary + library in one package?

Not yet, supporting both lib and bin will complicate bindings detection and our auditwheel Rust implementation, but it's been worked on slowly.

@not-my-profile
Copy link
Contributor

not-my-profile commented Jan 26, 2023

I really think that this is too soon ... we have not even stabilized the Rust API ... I have been refactoring our Rust API a lot recently but it's still very much not ready to be used by any third party, since it's very much subject to change drastically.

@charliermarsh
Copy link
Member

Yeah, I agree -- I was excited for this but I've realized that it's too soon. Let's close for now, since it's getting a bit stale anyway.

@squiddy squiddy deleted the python-library branch January 27, 2023 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants